Conversation
🦋 Changeset detectedLatest commit: 2177d34 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
packages/kit/src/vite/index.js
Outdated
| vite_config_env = config_env; | ||
| svelte_config = await load_config(); | ||
| is_build = config_env.command === 'build'; | ||
| completed_build = false; |
There was a problem hiding this comment.
Dominik mentioned build --watch, I think it should be in buildStart for that to work.
There was a problem hiding this comment.
There was a problem hiding this comment.
I don't understand. When the server restarts in that stackblitz, I see config called. @dominikg apparently mentioned something and I see @bluwy gave a thumbs up as well. What am I missing?
To me, config is a better place to put it because it happens sooner and we're already setting / resetting other variables there. If we set everything at the very beginning it saves us from having to worry about whether we've done it before the variable is used.
There was a problem hiding this comment.
When the server restarts in that stackblitz, I see config called.
The server doesn't restart in build --watch, config is only called when the plugin is loaded initially and we're already setting it to false where it's declared.
Let's say a build succeeds, the user then makes a change to some page which causes a build error, but config isn't called again so the flag isn't reset and it tries to go ahead with adapting in that case even though it shouldn't.
There was a problem hiding this comment.
Oh, I see. The stackblitz isn't using build --watch 😆
There was a problem hiding this comment.
Oop, yeah mb, couldn't get it working according to their docs for startCommand, just realized I coulda just changed the dev script to build --watch.
3 participants
I missed this while reviewing #5536 the first time around